Skip to content

fix: optimize fullscreen launcher item sorting initialization#709

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
wjyrich:fix-full-layer
Feb 26, 2026
Merged

fix: optimize fullscreen launcher item sorting initialization#709
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
wjyrich:fix-full-layer

Conversation

@wjyrich

@wjyrich wjyrich commented Feb 26, 2026

Copy link
Copy Markdown
Contributor
  1. Replaced Component.onCompleted with sortColumn property in FullscreenFrame.qml for ItemArrangementProxyModel
  2. Added setSortColumn method to SortProxyModel to enable declarative sorting in QML
  3. Modified SortProxyModel to reorder items when sortColumn changes if source model is already populated
  4. Added conditional enabling of itemMoveTransition in FullscreenFrame grid view Component.onCompleted

Log: Improved fullscreen launcher item sorting performance and stability

fix: 优化全屏启动器项目排序初始化

  1. 在 FullscreenFrame.qml 中将 ItemArrangementProxyModel 的 Component.onCompleted 替换为 sortColumn 属性
  2. 为 SortProxyModel 添加 setSortColumn 方法以支持在 QML 中声明式排序
  3. 修改 SortProxyModel 在 sortColumn 变化时重新排序项目(如果源模型已 填充)
  4. 在 FullscreenFrame 网格视图的 Component.onCompleted 中添加条件启用

sort(0)时,reorder() 会发出 beginMoveRows/endMoveRows 信号,与 GridView 的 displaced/move 动画冲突

根本解决方案:让排序在 setSourceModel() 的 beginResetModel/endResetModel 阶段就完成,这样视图第一次看到数据时就已经是排好序的,不会产生任何 move 信号。

做法:给 SortProxyModel 增加 sortColumn 的 Q_PROPERTY,在 QML 中声明式地设置 sortColumn: 0。QML 引擎会先设置简单值属性(sortColumn、sortRole),再设置复杂对象属性(sourceModel)。这样 rebuildRowMap() 执行m_sortColumn 已经是 0,数据在 reset 阶段就排好了。
Log: 改进全屏启动器项目排序性能和稳定性

Summary by Sourcery

Enable declarative initial sorting for fullscreen launcher items and avoid animation conflicts during model initialization.

New Features:

  • Allow SortProxyModel sorting to be configured via a declarative sortColumn property in QML.

Bug Fixes:

  • Prevent GridView move animations from conflicting with initial item sorting in the fullscreen launcher by sorting during model reset and conditionally enabling item move transitions.

Enhancements:

  • Trigger SortProxyModel to reorder items automatically when the sortColumn changes after the source model is populated.

@sourcery-ai

sourcery-ai Bot commented Feb 26, 2026

Copy link
Copy Markdown

Reviewer's Guide

Enables declarative initial sorting in the fullscreen launcher by introducing a sortColumn Q_PROPERTY on SortProxyModel, wiring it from QML, and ensuring resorting occurs when sortColumn changes, while conditionally enabling item move animations to avoid conflicts with model move signals.

Sequence diagram for declarative initial sorting of fullscreen launcher items

sequenceDiagram
    actor User
    participant QML_Engine
    participant FullscreenFrame_QML
    participant ItemArrangementProxyModel_QML
    participant SortProxyModel
    participant SourceModel
    participant GridView

    User->>QML_Engine: Open fullscreen launcher
    QML_Engine->>FullscreenFrame_QML: Instantiate FullscreenFrame
    FullscreenFrame_QML->>ItemArrangementProxyModel_QML: Create ItemArrangementProxyModel
    ItemArrangementProxyModel_QML->>SortProxyModel: Create SortProxyModel instance

    QML_Engine->>SortProxyModel: setSortRole(IndexInPageRole)
    QML_Engine->>SortProxyModel: setSortColumn(0)
    SortProxyModel->>SortProxyModel: store m_sortColumn = 0

    QML_Engine->>SortProxyModel: setSourceModel(SourceModel)
    SortProxyModel->>SourceModel: setSourceModel
    SourceModel-->>SortProxyModel: beginResetModel
    SortProxyModel->>SortProxyModel: rebuildRowMap using m_sortColumn
    SourceModel-->>SortProxyModel: endResetModel

    SortProxyModel-->>GridView: expose already sorted data
    GridView-->>User: Display items sorted with no move animations

    Note over SortProxyModel,GridView: No beginMoveRows or endMoveRows emitted during initial sort
Loading

Sequence diagram for runtime change of sortColumn triggering reorder

sequenceDiagram
    participant QML_Code
    participant SortProxyModel
    participant SourceModel
    participant GridView

    QML_Code->>SortProxyModel: setSortColumn(newColumn)
    SortProxyModel->>SortProxyModel: compare m_sortColumn with newColumn
    alt sortColumn changed and proxy map populated
        SortProxyModel->>SortProxyModel: update m_sortColumn
        SortProxyModel-->>QML_Code: sortColumnChanged signal
        SortProxyModel->>SortProxyModel: reorder()
        SortProxyModel->>GridView: beginMoveRows signals
        SortProxyModel->>GridView: endMoveRows signals
        GridView->>GridView: run itemMoveTransition
    else no change or no data
        SortProxyModel-->>QML_Code: no reordering
    end
Loading

Sequence diagram for conditional enabling of itemMoveTransition in FullscreenFrame

sequenceDiagram
    participant FullscreenFrame_QML
    participant GridView
    participant LauncherController

    FullscreenFrame_QML->>GridView: Component.onCompleted
    GridView->>LauncherController: read currentFrame
    LauncherController-->>GridView: currentFrame value
    alt currentFrame is FullscreenFrame
        GridView->>GridView: itemMoveTransition.enabled = true
    else currentFrame is not FullscreenFrame
        GridView->>GridView: itemMoveTransition remains disabled
    end
    GridView->>GridView: gridViewContainer.checkPageSwitchState()
Loading

Class diagram for updated SortProxyModel with sortColumn Q_PROPERTY

classDiagram
    class QAbstractProxyModel

    class SortProxyModel {
        <<QML_ELEMENT>>
        - int m_sortRole
        - int m_sortColumn
        - Qt_CaseSensitivity m_sortCaseSensitivity
        + int sortRole()
        + void setSortRole(int role)
        + int sortColumn()
        + void setSortColumn(int column)
        + Qt_CaseSensitivity sortCaseSensitivity()
        + void setSortCaseSensitivity(Qt_CaseSensitivity sensitivity)
        + void setSourceModel(QAbstractItemModel model)
        + void reorder()
        + void sortRoleChanged()
        + void sortColumnChanged()
    }

    SortProxyModel --|> QAbstractProxyModel
Loading

File-Level Changes

Change Details Files
Add declarative sortColumn support to SortProxyModel and trigger reordering when it changes after the source model is populated.
  • Introduce sortColumn as a Q_PROPERTY on SortProxyModel with associated NOTIFY signal for QML bindings.
  • Implement setSortColumn(int) to update the internal sort column, emit sortColumnChanged, and call reorder() when a source model with an existing proxy map is present.
  • Update class documentation comment to describe declarative initial sorting behavior via sortColumn.
src/models/sortproxymodel.h
src/models/sortproxymodel.cpp
Use sortColumn from QML to ensure items are sorted during model reset and avoid animation conflicts, and gate itemMoveTransition enabling on the fullscreen frame being active.
  • Replace the late Component.onCompleted call to proxyModel.sort(0) with declarative sortColumn: 0 on ItemArrangementProxyModel to let sorting happen during model reset rather than via move signals.
  • Add a conditional check on LauncherController.currentFrame before enabling itemMoveTransition in the grid view's Component.onCompleted handler to avoid unnecessary move animations outside the fullscreen frame context.
  • Keep existing lifecycle hooks (onCompleted/onDestruction) for gridViewContainer while integrating the new conditional animation enabling.
qml/FullscreenFrame.qml

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've left some high level feedback:

  • In SortProxyModel::setSortColumn, the oldColumn variable is never used except for Q_UNUSED(oldColumn); you can drop the temporary and the Q_UNUSED call entirely to simplify the setter.
  • The Component.onCompleted handler that conditionally enables itemMoveTransition based on LauncherController.currentFrame only runs once; if currentFrame can change later, consider binding itemMoveTransition.enabled to an expression or reacting to currentFrame changes so the transition state stays in sync.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `SortProxyModel::setSortColumn`, the `oldColumn` variable is never used except for `Q_UNUSED(oldColumn)`; you can drop the temporary and the `Q_UNUSED` call entirely to simplify the setter.
- The `Component.onCompleted` handler that conditionally enables `itemMoveTransition` based on `LauncherController.currentFrame` only runs once; if `currentFrame` can change later, consider binding `itemMoveTransition.enabled` to an expression or reacting to `currentFrame` changes so the transition state stays in sync.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread qml/FullscreenFrame.qml Outdated
Comment thread src/models/sortproxymodel.h Outdated
1. Replaced Component.onCompleted with sortColumn property in
FullscreenFrame.qml for ItemArrangementProxyModel
2. Added setSortColumn method to SortProxyModel to enable declarative
sorting in QML
3. Modified SortProxyModel to reorder items when sortColumn changes if
source model is already populated
4. Added conditional enabling of itemMoveTransition in FullscreenFrame
grid view Component.onCompleted

Log: Improved fullscreen launcher item sorting performance and stability

fix: 优化全屏启动器项目排序初始化

1. 在 FullscreenFrame.qml 中将 ItemArrangementProxyModel 的
Component.onCompleted 替换为 sortColumn 属性
2. 为 SortProxyModel 添加 setSortColumn 方法以支持在 QML 中声明式排序
3. 修改 SortProxyModel 在 sortColumn 变化时重新排序项目(如果源模型已
填充)
4. 在 FullscreenFrame 网格视图的 Component.onCompleted 中添加条件启用

sort(0)时,reorder() 会发出 beginMoveRows/endMoveRows 信号,与 GridView 的 displaced/move 动画冲突

根本解决方案:让排序在 setSourceModel() 的 beginResetModel/endResetModel 阶段就完成,这样视图第一次看到数据时就已经是排好序的,不会产生任何 move 信号。

做法:给 SortProxyModel 增加 sortColumn 的 Q_PROPERTY,在 QML 中声明式地设置 sortColumn: 0。QML 引擎会先设置简单值属性(sortColumn、sortRole),再设置复杂对象属性(sourceModel)。这样 rebuildRowMap() 执行m_sortColumn 已经是 0,数据在 reset 阶段就排好了。
Log: 改进全屏启动器项目排序性能和稳定性
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

这段代码 diff 展示了对 SortProxyModel 类的修改,主要目的是在 QML 中支持通过属性声明的方式设置排序列,而不是在 Component.onCompleted 中手动调用 sort() 方法。同时,在 FullscreenFrame.qml 中增加了对当前 Frame 的判断逻辑。

以下是对这段代码的详细审查和改进意见:

1. 语法与逻辑审查

  • QML 逻辑改进 (FullscreenFrame.qml):

    • 原代码: Component.onCompleted: { proxyModel.sort(0) }
    • 新代码: sortColumn: 0
    • 分析: 这是一个很好的改进。将命令式的 sort() 调用改为声明式的属性绑定,更符合 QML 的编程范式。根据头文件 sortproxymodel.h 中的注释,这样做可以在模型重置期间启用初始排序,且不发出移动信号,这通常能减少不必要的视图重绘和动画,提升性能。
  • C++ 逻辑改进 (sortproxymodel.cpp):

    • 新增方法: setSortColumn(int column)
    • 分析: 逻辑实现正确。它检查了值是否变化 (m_sortColumn != column),更新了成员变量,发射了信号,并在源模型已设置的情况下触发了 reorder()
    • 潜在问题: Q_UNUSED(oldColumn) 这一行代码虽然是为了消除编译器关于未使用变量的警告,但 oldColumn 变量本身在这个上下文中确实没有实际用途(不需要根据旧列做特殊处理)。保留它可能是为了调试或未来扩展,但在当前逻辑下显得多余。建议直接移除 oldColumn 的定义。
  • QML 条件判断 (FullscreenFrame.qml):

    • 新增代码:
      if (LauncherController.currentFrame === "FullscreenFrame") {
          itemMoveTransition.enabled = true
      }
    • 分析: 这段逻辑放在 Component.onCompleted 中,意味着组件创建完成时检查一次。如果 LauncherController.currentFrame 在组件生命周期内发生变化,这段代码不会响应。如果这是预期行为(仅初始化时检查),则没有问题。如果需要动态响应,应该使用属性绑定。

2. 代码质量

  • 命名规范: sortColumnsetSortColumn 命名清晰,符合 Qt 和 C++ 的命名规范。
  • 代码注释: 头文件中的注释 @note sortColumn can be set declaratively in QML... 非常有用,解释了修改的意图和行为。
  • 变量作用域: 如前所述,oldColumn 变量在 setSortColumn 中定义后未使用,降低了代码的整洁度。

3. 代码性能

  • 排序触发时机:
    • setSortColumn 中,如果 sourceModel() 存在且映射非空,会调用 reorder()。这意味着每次设置 sortColumn 都会立即触发排序。
    • 在 QML 中使用 sortColumn: 0 时,这会在对象构造期间发生。相比 Component.onCompleted,这可能会让排序发生得稍早一些,但通常不会带来显著的性能差异。
    • 注意: 如果 reorder() 是一个耗时操作,且在模型数据量很大时频繁调用 setSortColumn,可能会导致界面卡顿。目前的实现通过 if (m_sortColumn != column) 增加了一个守卫,避免了不必要的重复排序,这是很好的做法。

4. 代码安全

  • 空指针检查: if (sourceModel() && !m_proxyToSourceMap.empty()) 做了必要的空指针和状态检查,防止了潜在的空指针解引用崩溃。
  • 类型安全: 参数类型为 int,与 sortColumn 属性类型一致,没有明显的类型安全问题。

改进建议

  1. 移除未使用的变量 (sortproxymodel.cpp):
    建议移除 oldColumn 变量,因为它既没有被使用,也没有在逻辑上提供帮助。这会让代码更简洁。

    // 改进后的 setSortColumn
    void SortProxyModel::setSortColumn(int column)
    {
        if (m_sortColumn != column)
        {
            m_sortColumn = column;
            Q_EMIT sortColumnChanged();
    
            // If source model is already set, reorder to apply the new sort column
            if (sourceModel() && !m_proxyToSourceMap.empty())
            {
                reorder();
            }
        }
    }
  2. QML 属性绑定 (FullscreenFrame.qml):
    对于 itemMoveTransition.enabled 的设置,如果逻辑上需要它始终跟随 LauncherController.currentFrame 的状态,建议使用绑定代替命令式赋值:

    // 建议改为属性绑定(如果业务逻辑允许)
    itemMoveTransition.enabled: LauncherController.currentFrame === "FullscreenFrame"

    如果必须放在 onCompleted 中(例如只需要初始化一次),请确保添加注释说明原因,以免后续维护者误以为是遗漏的绑定。

  3. 文档完善 (sortproxymodel.h):
    虽然添加了 @note,但可以进一步明确 sortColumn 的默认值(如果有的话),以及它与 sortRole 的关系(例如:sortRole 决定按什么数据排序,sortColumn 决定按模型中的哪一列排序,尽管对于 QAbstractListModel 这种单列模型,column 通常为 0)。

总结

这段代码修改整体质量较高,成功地将命令式调用转换为声明式属性,提升了代码的可维护性和 QML 风格的一致性。逻辑上安全,考虑了边界条件。主要的改进点在于清理未使用的变量以及考虑 QML 端是否可以使用属性绑定来替代 onCompleted 中的条件判断。

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: BLumia, wjyrich

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@wjyrich

wjyrich commented Feb 26, 2026

Copy link
Copy Markdown
Contributor Author

/forcemerge

@deepin-bot

deepin-bot Bot commented Feb 26, 2026

Copy link
Copy Markdown

This pr force merged! (status: blocked)

@deepin-bot deepin-bot Bot merged commit b178a28 into linuxdeepin:master Feb 26, 2026
6 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants